feat: UIKit Navigation instrimentation #22
Conversation
arriIsHere
force-pushed
the
arriIsHere.ui-navigation-controller-instrumentation
branch
2 times, most recently
from
e6fdbcc to
984b43f
Compare
| @@ -65,6 +65,15 @@ struct ContentView: View { | |||
| NetworkView() | |||
| .padding() | |||
| .tabItem { Label("Network", systemImage: "network") } | |||
|
|
|||
| UIKView() | |||
There was a problem hiding this comment.
The name UIKView feels very wrong to me. Objective C didn't have namespaces, so the convention was to use two letter (and later, three letter) prefixes to avoid naming collisions. "UI" was the prefix for UIKit. My brain can't help but read UIKView as a View in the UIK namespace. I'd recommend just making it UIKitView.
| let app = XCUIApplication() | ||
| app.launch() | ||
| app.buttons["UIKit"].tap() | ||
| XCTAssert(app.staticTexts["Sample UIKit App"].waitForExistence(timeout: uiUpdateTimeout)) |
There was a problem hiding this comment.
Since we're swizzling UIViewController, this instrumentation should work for both UINavigationContoller and UITabBarController, among others. Should we add tests for both a navigation controller and a tab view, to make sure they're both reasonable? I'm not sure how annoying that would be to build in Storyboard.
| import OpenTelemetryApi | ||
| import UIKit | ||
|
|
||
| private let honeycombInstrumentationView = "@honeycombio/instrumented-view" |
There was a problem hiding this comment.
PR #20 is probably gonna go in before this one, and has "@honeycombio/instrumentation-view". Let's find a place where we can put it as a constant and reuse it. Maybe all of our tracer names should be in one file? 🤔
arriIsHere
force-pushed
the
arriIsHere.ui-navigation-controller-instrumentation
branch
from
5d68eb6 to
ef672fe
Compare
arriIsHere
force-pushed
the
arriIsHere.ui-navigation-controller-instrumentation
branch
from
ef672fe to
a11a82e
Compare
| // SmokeTest | ||
| // | ||
| // Created by Arri Blais on 11/13/24. | ||
| // |
There was a problem hiding this comment.
You should delete these headers. You can change the template in Xcode settings somewhere.
README.md
Outdated
| @@ -76,6 +76,16 @@ To manually send a span: | |||
| The following auto-instrumentation libraries are automatically included: | |||
| * [MetricKit](https://developer.apple.com/documentation/metrickit) data is automatically collected. | |||
|
|
|||
| ### UIKit instrumentation | |||
There was a problem hiding this comment.
nit: It looks like we capitalized "Instrumentation" everywhere else, so let's be consistent.
arriIsHere
force-pushed
the
arriIsHere.ui-navigation-controller-instrumentation
branch
from
16bd1cb to
8586f74
Compare
arriIsHere
deleted the
arriIsHere.ui-navigation-controller-instrumentation
branch
Which problem is this PR solving?
Adds auto-instrimentation for UIKit navigation.
Short description of the changes
Swizzles the UIViewController
viewDidAppearandviewDidDisappearto observeHow to verify that this has the expected result
Some smoke tests were added that verify the collection of data (WIP needs to be more granular)